Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow options via query. #104

Merged
merged 2 commits into from
Oct 7, 2016

Conversation

izaakschroeder
Copy link
Contributor

When webpack@2 lands there will no longer be any this.options. Since query can now be a vanilla object instead of a string, having options reside in query is a quick, non-destructive fix.

See:

When `webpack@2` lands there will no longer be any `this.options`. Since query can now be a vanilla object instead of a string, having `options` reside in query is a quick, non-destructive fix.
@ai
Copy link
Contributor

ai commented Oct 7, 2016

Why extra option instead of having option in root level in query?

@izaakschroeder
Copy link
Contributor Author

The query itself must either be object or string and it's nice to have options as a function. It's also the least intrusive change.

@ai
Copy link
Contributor

ai commented Oct 7, 2016

Maybe we should name it as plugins?

@izaakschroeder
Copy link
Contributor Author

izaakschroeder commented Oct 7, 2016

Yea we could do query.plugins instead of query.options; that works. 👍

@ai ai merged commit 7786661 into webpack-contrib:master Oct 7, 2016
@izaakschroeder izaakschroeder deleted the options-via-query branch October 7, 2016 17:30
@izaakschroeder
Copy link
Contributor Author

izaakschroeder commented Oct 7, 2016

Thanks! Let me know when a new version is published @ai 😄

@ai
Copy link
Contributor

ai commented Oct 7, 2016

I just need to update docs, add result.messages with dependencies and we are ready for release.

@izaakschroeder
Copy link
Contributor Author

👌

@ai
Copy link
Contributor

ai commented Oct 8, 2016

I finished all changes and ready for release. Could you test this plugin on real application? (Just replace version in package.json to postcss/postcss-loader)

@michael-ciniawsky
Copy link
Member

webpack v2.1.0-beta.25

LoaderOptionsPlugin: ✅
postcss.config.js: 🚫

I'm on it... :)

@izaakschroeder
Copy link
Contributor Author

Will try and get this working will my configuration tonight 😄

@izaakschroeder
Copy link
Contributor Author

@ai Running into a few hiccups. First of which is ./node_modules/postcss-load-config/index.js. There's some mismatch between versions going on here, where that file is trying to require postcss-load-options/lib/loadOptions.js which doesn't exist; the file is instead called postcss-load-options/lib/options.js. I'm not sure which version is intended to be correct or where the mismatch is coming from.

@izaakschroeder
Copy link
Contributor Author

izaakschroeder commented Oct 12, 2016

@ai The way you've refactored parseOptions also results in failing to pass the webpack context through.

return parseOptions(options, pack);

should be:

return parseOptions(loader, options, pack);

and then:

function parseOptions(context, options, pack) {
// ...
options = options.call(context, context);

This will break postcss-imports (and possibly more) otherwise.

@izaakschroeder
Copy link
Contributor Author

You can verify these by checking out the branch here: webpack-config/webpack-config-postcss#10 and running npm run smoke.

@michael-ciniawsky
Copy link
Member

@izaakschroeder postcss-load-config issue should be fixed now.

@izaakschroeder
Copy link
Contributor Author

Thanks @michael-ciniawsky it is 😄 Just waiting on @ai now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants